[US-16.2 / OBT-253] Exclude platform admins from the project Grant User picker - #37
Open
levigtri wants to merge 3 commits into
Open
Conversation
Optional prop to drop platform admins from the search results, folded into the existing excludeIds filter. Generalize the empty-results message so it reads correctly whether matches were dropped for being already-added or for being admins.
Platform admins already manage every project, so they can't be added as members. Enable excludePlatformAdmins on the Grant User picker, and surface the backend's 400 rejection with a clear toast as defense in depth.
📝 WalkthroughWalkthrough
ChangesPlatform Admin Access
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/common/UserSearchPicker.tsx (1)
73-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the filtering contract.
Test mixed results, all-platform-admin results, and existing-user exclusion so the new empty-state behavior cannot regress.
Also applies to: 136-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/common/UserSearchPicker.tsx` around lines 73 - 77, Add regression tests for the filtering logic in UserSearchPicker, covering mixed results, results consisting entirely of platform admins, and exclusion of IDs already present in excludeIds. Assert that each scenario produces the expected filtered results and empty state, preserving the contract implemented by the results.filter predicate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/common/UserSearchPicker.tsx`:
- Around line 73-77: Add regression tests for the filtering logic in
UserSearchPicker, covering mixed results, results consisting entirely of
platform admins, and exclusion of IDs already present in excludeIds. Assert that
each scenario produces the expected filtered results and empty state, preserving
the contract implemented by the results.filter predicate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Pro Plus
Run ID: d5712728-0027-4622-a3bb-ba257f495851
📒 Files selected for processing (2)
src/components/common/UserSearchPicker.tsxsrc/components/pages/ProjectAccessTab.tsx
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[US-16.2 / OBT-253] Exclude platform admins from the project Grant User picker
Summary
Platform Admin accounts must not be offered when granting access to a project — they already manage every project implicitly, so they can't be added as a member or given a project role. The "Grant User Access" dialog on a project's Access tab uses
UserSearchPicker, which previously filtered results only byexcludeIds(users already granted). This change teaches the picker to drop platform admins from its results, and wires that on in the project Access tab. The user-search response (UserListResponse) already carriesis_platform_admin, so no new endpoint is needed. The already-granted members table is unaffected because the backend (US-16.1, shemaobt/tripod-api#…) now excludes admins from the listing. As defense in depth, the grant handler also surfaces the backend's 400 rejection with a clear toast, in case an admin is reached some other way.Changes
src/components/common/UserSearchPicker.tsx— new optionalexcludePlatformAdminsprop. When set, search results withis_platform_adminare filtered out (alongside the existingexcludeIdsfilter, now folded into a singleresults.filter). The empty-results message is generalized to "All matching users are already added or unavailable" so it reads correctly whether matches were dropped for being already-added or for being admins.src/components/pages/ProjectAccessTab.tsx— passesexcludePlatformAdminsto the Grant User picker, and adds a400branch tohandleGrantUserthat toasts "Platform admins can't be added to a project" (defense in depth; the picker already prevents selecting one).Type of Change
Testing
npm run typecheck,npm run lint(0 errors; only the pre-existing ProjectsPage warning) andnpm run buildpass.Summary by CodeRabbit
New Features
Bug Fixes